Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: remove --disable-component-update from default args #3298

Merged
merged 1 commit into from
Nov 19, 2024
Merged

feat: remove --disable-component-update from default args #3298

merged 1 commit into from
Nov 19, 2024

Conversation

andrea11
Copy link
Contributor

@andrea11 andrea11 commented Nov 13, 2024 •
edited
Loading

Similarly to puppeteer/puppeteer#13010,
Quoting @mathiasbynens:

Summarizing the “why” here:
The --disable-component-update command-line flag does two things: it disables component updates (desired) but it also prevents any bundled components from being initialised (undesired).

Disable this flag grant more flexibility, and it can always be disabled via the web-ext run --arg option

@andrea11 andrea11 mentioned this pull request Nov 13, 2024
Merged
@andrea11
Copy link
Contributor Author

Can someone review this PR? (@willdurand maybe?)

Rob--W
Rob--W approved these changes Nov 14, 2024
View reviewed changes
Copy link
Member

@Rob--W Rob--W left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to not auto-download a bunch of extra stuff, but since it is currently possible to add it again, but not to opt out, I reluctantly approve this change.

When merged, this will fix #3093

@willdurand
Copy link
Member

I'd prefer to not auto-download a bunch of extra stuff, but since it is currently possible to add it again, but not to opt out, I reluctantly approve this change.

I am not clear on what that means, @Rob--W.

@Rob--W
Copy link
Member

Rob--W commented Nov 18, 2024

I'd prefer to not auto-download a bunch of extra stuff, but since it is currently possible to add it again, but not to opt out, I reluctantly approve this change.

I am not clear on what that means, @Rob--W.

I am approving this change, even if it means that Chrome will auto-download some additional components after startup. This will utilize extra bandwidth and CPU when launching (new) browser profiles instances. In exchange for that downside, the upside is that the launched Chrome browser will more closely match production. One example of that is that websites requiring DRM will work.

If devs want to revert to the behavior from before, they can manually add the command line flag to web-ext run.

TL;DR: LGTM, let's merge.

@willdurand willdurand merged commit b03f190 into mozilla:master Nov 19, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Rob--W Rob--W Rob--W approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@andrea11 @willdurand @Rob--W